SDK-470 Add CustomDebugStringConvertible to AuthFailureReason#1073
Conversation
AuthFailureReason is an Int-backed @objc enum, so logging an instance prints a raw number. Conform to CustomDebugStringConvertible so logs show the case name instead. The switch is exhaustive with no default, so adding a future case fails to compile until its label is added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1073 +/- ##
==========================================
+ Coverage 71.58% 71.60% +0.01%
==========================================
Files 113 114 +1
Lines 9438 9462 +24
==========================================
+ Hits 6756 6775 +19
- Misses 2682 2687 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| case authTokenMissing | ||
| } | ||
|
|
||
| extension AuthFailureReason: CustomDebugStringConvertible { |
There was a problem hiding this comment.
We already solve this exact problem for IterableInAppContentType and IterableInAppTriggerType in InAppPersistence.swift using CustomStringConvertible and description, with a comment explaining why it's needed for @objc enums.
Should we stay consistent and use CustomStringConvertible + description here too?
Both protocols technically work for String(describing:) thanks to the fallback chain, but description is the right semantic for output customers will see in their AuthDelegate logs, and it matches the existing convention in this SDK.
There was a problem hiding this comment.
Fair point, and the InAppPersistence precedent is useful context. Keeping CustomDebugStringConvertible here since SDK-470 asks for it specifically. It still covers the customer path: print(reason) and "\(reason)" go through String(describing:), which falls back to debugDescription, so AuthDelegate logs show the case name. Updated the test (your other comment) to assert String(describing: reason) so it guards that path.
| ] | ||
|
|
||
| expectedDescriptions.forEach { reason, description in | ||
| XCTAssertEqual(reason.debugDescription, description) |
There was a problem hiding this comment.
This is asserting debugDescription against the same strings we just hardcoded in the switch, so it can't fail in a meaningful way.
The bug we're fixing is that print(reason) or "\(reason)" shows AuthFailureReason(rawValue: 0) instead of the case name.
Can we switch this to XCTAssertEqual("\(reason)", description) (or String(describing: reason)) so the test actually covers the customer-facing logging path?
That way if someone changes the protocol in the future and breaks String(describing:), this test catches it.
There was a problem hiding this comment.
Good call. Switched the assertion to String(describing: reason) so it covers the print(reason) / "\(reason)" path instead of debugDescription directly.
Assert the customer-facing logging output instead of debugDescription directly. print(reason) and "\(reason)" resolve through String(describing:), which falls back to the enum debugDescription, so this guards the case-name output customers see in AuthDelegate logs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
3 participants
What
AuthFailureReasonis an Int-backed@objcenum, so logging an instance prints a raw number. This makes it conform toCustomDebugStringConvertibleso logs show the case name.Changes
CustomDebugStringConvertibleextension mapping each case to its name.default, so a future case fails to compile until labeled.Impact
Testing
How to test:
./agent_test.sh AuthTests.testAuthFailureReasonDebugDescriptionspasses for all 11 cases.String(reflecting: AuthFailureReason.authTokenExpired)returns"authTokenExpired".Jira: https://iterable.atlassian.net/browse/SDK-470